program: use counter to access accounts list #17
Conversation
|
@joncinque I'm leaving this as a draft for now, but let me know what you think about my findings detailed above. I've also included a test with comments to demonstrate what I'm talking about. |
joncinque
left a comment
joncinque
left a comment
There was a problem hiding this comment.
Nice catch! For the first BPF version of these programs, I think we should stick with replicating all the behavior of the native program, unless it's an obvious bug. We can change it in future work
buffalojoec
force-pushed
the
loop-counter-bug
branch
from
f939197 to
9f39d6c
Compare
|
OK, I rebased with real commits this time, so it can come out of draft, and ready for another look. Side note: the compute unit profiling actually helped me catch a bug! The |
That's very interesting! We should definitely keep that in mind for optimized programs |
buffalojoec
changed the title
Problem
When initializing a config account, the config account is expected to be a signer. However, if it's also included in the signer list as a required signer, the loop where signer keys are checked will fail, telling the user that the account is missing. This failure case also arises when updating a config account, and attempting to declare that the config account must be a signer for updates.
The account isn't actually missing, it's just the fact that the
if signer != config_account_keyis being evaluated after the counter is advanced, so the rest of the loop is off by one.https://github.com/anza-xyz/agave/blob/d9c24ed5837407aa411fbb7cc881c4a1a9757f5e/programs/config/src/config_processor.rs#L61-L72
In the BPF version, we properly handle this case by only advancing the
accounts_iterwhen the list of accounts is actually accessed.In my opinion, this seems like a bug with the builtin implementation, not the BPF version. It's possible that maybe the program isn't designed to allow you to include the config account as a required signer, but that seems like an unnecessary constraint.